Polish chat UI and provider prompts#2451
Conversation
… remove TODO.md - Model picker, provider settings, traits, badges, timestamps, branch/workspace pickers - Bundled fonts; context menu fallback reliability; DONE.md changelog - Delete TODO.md; server/provider contract and text-generation related updates Co-authored-by: Cursor <cursoragent@cursor.com>
Co-authored-by: Cursor <cursoragent@cursor.com>
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. 👉 Get your free trial and get 200 agent minutes per Slack user (a $50 value). Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
github-actions
Bot
added
size:XXL
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: c967b6d2ed
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| onPromptChange(stripped); | ||
| } | ||
| updateDescriptors(replaceDescriptorCurrentValue(descriptors, descriptor.id, value)); | ||
| updateDescriptors(replaceDescriptorCurrentValue(visibleDescriptors, descriptor.id, value)); |
There was a problem hiding this comment.
When hiddenDescriptorIds is used, replaceDescriptorCurrentValue is called with visibleDescriptors only, so changing a single trait rebuilds modelOptions from a partial descriptor list and drops every hidden option. This is reachable in the new split-trigger flow (each popup hides sibling descriptors), and it can silently reset previously selected provider options (e.g., reasoning/context settings) on the next change.
Useful? React with 👍 / 👎.
| <TooltipTrigger | ||
| render={<p className="text-right text-muted-foreground text-sm" />} | ||
| > | ||
| {formatChatTimestamp(row.message.createdAt)} |
There was a problem hiding this comment.
This row now renders relative time via formatChatTimestamp(...), but non-streaming message rows have no ticking state, so labels like 59s ago stop updating until an unrelated re-render happens. The new chat timestamp UX therefore becomes stale during idle sessions and can mislead users about message recency.
Useful? React with 👍 / 👎.
|
Fixed this group to centered vertically correctly... Old: New: |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
ApprovabilityVerdict: Needs human review 4 blocking correctness issues found. Diff is too large for automated approval analysis. A human reviewer should evaluate this PR. You can customize Macroscope's approvability policy. Learn more. |
|
Thank you so much for this work! Going through it all and will leave my thoughts as I look at each part Biggest thing: There's a regression on starting new threads where the chat view gets populated, then a big visible repaint happens before the thread actually starts moving. Def need that fixed before we can ship. Here's me going through each part:
12.1. I don't like the separated "Command + number" on MacOS, feels...weird. Might want to rethink the shortcut component at this point 🙃 
12.2. We should do some kind of callout on "why you can't switch providers" here so users aren't as confused. Out of scope for this just writing it down before I forget to
28-31 I don't love the "selected" badge, I feel like badges are for different types of context and it's weird using them this way. I can be convinced but would also love to think of other treatments. The check we currently use on the permissions is decent? Not sure.
Also - do tool call summaries use inference? Not sure how I feel about that being on by default. Generally I think people care too much about tool calls and we were intentional with our decision to make them look uninteresting Overall this is great and I can't wait to merge it |
|
I like a lot of it, but removing the initials from the providers is a big no. I don't remember which color I chose for each of my codex accounts, but i do now i named them Work and Personal, so CW/CP is very useful information to me. Also agree with Theo on the overuse of badges for selected state. checkmarks are fine? the sensitive column is waaaay too big as well:
I think this was to prevent layout shifts as it counts up |
|
@juliusmarminge @t3dotgg I've applied to the fixes to the issues raised. I also added the option to answer to questionnaires with your own input, as it was low effort and probably a highly requested feature.
I dont have this issue. Is it a mac issue? It loads instantly for me without any repaints. I also didn't touch anything about the shortcut component yet, I'm not sure what todo about it so I'm leaving it as. Questionnaire
Added back the initials for coloursSomewhat reckless in hindsight to remove this. Requested by Julius 
Limited the tool calls visible to 1 at a timeI made the tool calls show the latest tool call currently available. You can still expand it down. 
Toned down the glow for when text area is focusedI also thought it was too much, but didn't want to lose the intent with the focus, so I just made it consistent with what other inputs have as focus ring, but now it's toned down. 
Moved checkbox to the right
Debadged most of the UII agree that the badge usage was excessive and a checkmark is sufficient for most UI here. It now displays a blue checkmark. As requested by both 
Fixed both icons under the text-area to not look weird.The issue was that it was not properly sized with the text. As requested by Theo 
Updated model picker againAdded an explanation for why models are missing by adding them back as disabled, with a tooltip on hover. Also added a slight 1 tw unit offset to give it room to breathe. as requested by Theo Made tool call summaries default to offIt uses inference as Theo suspected, using the same model as the text generation model. I would come back to this and suggest to create another model picker in settings allowing a small model like gpt-5-nano to generate summaries. Requested by Theo 
Simplified visuals for the "worked for" dash
Removed the double highlight for buttons inside directory tab and per thread tab
Fixed revert button to be in the button group with copy prompt button
Preserved GitHub original colourDuring a pass to fix every icon to be consistently muted when it has a foreground text label, GPT accidentally overwrote GitHub's logo to be muted. I reverted this change to respect GitBub's branding guidelines. 
Fixed dialogs under Connections to use correct vertical centeringThe dialogs for Connections used the vertical centering for a command menu (like search bar). I changed it to be middle to create a distinction between user expectations and what shadcn does. 
Lowered blur tint for dialogsLowered from sm (8px) to xs (4px). Requested by Theo Refactored the changed files cardIt was using the old style and had too much visual noise. I simplified it while preserving all details, and added the +- diff to be a consistent 4ch. This should hopefully allow easier visual scanning. I also added rounding to closest power when it reaches 1000 LoC (very unlikely). Also fixed various padding issues around the card. 
Fixed visual profile of when a tool call errors with runtime errrorWhen a tool call errored like this, its visual profile was all over the place. I made it fully red, to create a difference between a runtime error and normal error. 
Added blue checkmark when a model selected
Fixed top padding for on the model rail for model pickerIt was off by a pixel, so I fixed it. I also did a lot of misc like fixing alignment and padding which was not important enough to write here, but I added my notes just in-case: Fixed inconsistent behaviour when Claude is unauthenticatedI noticed that Codex and Claude behave differently in T3 Code when unauthenticated, where Codex generates an alert banner. I made this the same behaviour for Claude also. |
|
Some more fixes |
…w assistant meta only on terminal message
|
|
as for the custom answer, you can do this today? and since we're using the same composer all features from it still works like file tagging etc: CleanShot.2026-05-03.at.12.45.02.mp4 |
…bar missing during answering - Always include totalProcessedTokens in context-window payload (was conditionally omitted when equal to usedTokens), fixing the status bar text disappearing during agent responses - Show provider display name in compaction text with bold formatting - Combine token usage and compaction into a single flowing paragraph - Add mt-2 spacing to status footer Fix lint warnings: unused var, map spread -> Object.assign, function scoping UI polish: preference picker layout, trait search, warning indicator styling, composer mode controls, empty state alignment, diff stat layout, work log failure markers
| return ( | ||
| <div | ||
| key={`${activeQuestion.id}:${option.label}`} | ||
| role="button" | ||
| tabIndex={isResponding ? -1 : 0} | ||
| aria-disabled={isResponding} | ||
| onClick={() => { | ||
| if (isResponding) return; | ||
| handleOptionSelection(activeQuestion.id, option.label); | ||
| }} | ||
| className={className} | ||
| > | ||
| {content} | ||
| </div> |
There was a problem hiding this comment.
🟠 High chat/ComposerPendingUserInputPanel.tsx:207
The change from <button> to <div role="button"> breaks keyboard activation. While the div has tabIndex={0} and role="button", it lacks an onKeyDown handler for Enter and Space keys. Users who tab to an option cannot activate it with the keyboard — the original <button> handled this natively.
- <div
+ <button
key={`${activeQuestion.id}:${option.label}`}
- role="button"
- tabIndex={isResponding ? -1 : 0}
- aria-disabled={isResponding}
+ type="button"
+ disabled={isResponding}
onClick={() => {
if (isResponding) return;
handleOptionSelection(activeQuestion.id, option.label);
}}
className={className}
>
{content}
- </div>
+ </button>🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/components/chat/ComposerPendingUserInputPanel.tsx around lines 207-220:
The change from `<button>` to `<div role="button">` breaks keyboard activation. While the `div` has `tabIndex={0}` and `role="button"`, it lacks an `onKeyDown` handler for Enter and Space keys. Users who tab to an option cannot activate it with the keyboard — the original `<button>` handled this natively.
Evidence trail:
apps/web/src/components/chat/ComposerPendingUserInputPanel.tsx lines 207-219 at REVIEWED_COMMIT: `<div role="button" tabIndex={...} onClick={...}>` with no `onKeyDown` handler. git_diff MERGE_BASE..REVIEWED_COMMIT shows original code used `<button type="button" onClick={...}>` which natively handles Enter/Space. WAI-ARIA Button Pattern: https://www.w3.org/WAI/ARIA/apg/patterns/button/
There was a problem hiding this comment.
🟡 Medium components/ChatView.tsx:2803
onSelectActivePendingUserInputOption clears the draft's customAnswer by calling togglePendingUserInputOptionSelection, but it no longer clears promptRef.current. When the user then sends a message, onSend reads promptRef.current directly as promptForSend and uses it for deriveComposerSendState and appendTerminalContextsToPrompt. This causes the stale custom answer text to be sent as a new message even though the composer visually appears empty.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/components/ChatView.tsx around line 2803:
`onSelectActivePendingUserInputOption` clears the draft's `customAnswer` by calling `togglePendingUserInputOptionSelection`, but it no longer clears `promptRef.current`. When the user then sends a message, `onSend` reads `promptRef.current` directly as `promptForSend` and uses it for `deriveComposerSendState` and `appendTerminalContextsToPrompt`. This causes the stale custom answer text to be sent as a new message even though the composer visually appears empty.
Evidence trail:
ChatView.tsx:2780-2810 - `onSelectActivePendingUserInputOption` does not clear `promptRef.current`; ChatView.tsx:2822 - `onChangeActivePendingUserInputCustomAnswer` sets `promptRef.current = value`; pendingUserInput.ts:76-101 - `togglePendingUserInputOptionSelection` returns `{customAnswer: ""}` clearing the draft but not the ref; ChatComposer.tsx:1943-1948 - editor value is `activePendingProgress.customAnswer ?? ""` when pending, `prompt` otherwise; ChatComposer.tsx:1117-1120 - sync effect only runs when draft store `prompt` changes; ChatComposer.tsx:1310-1327 - `onPromptChange` routes to pending handler (doesn't call `setPrompt`) so draft store `prompt` never changes during pending input; ChatView.tsx:2409-2412 - `onSend` early-returns when `activePendingProgress` is truthy; ChatView.tsx:2424 - `promptForSend = promptRef.current` when `activePendingProgress` is null
|
quite a bit of conflicts, mind resolving them? |
…ialog + markdown links
There was a problem hiding this comment.
🟢 Low
The removal of fileLinkParentSuffixByPath breaks disambiguation for files with identical basenames. When two links point to different files named MessagesTimeline.tsx in different directories, both now display as "MessagesTimeline.tsx" instead of "MessagesTimeline.tsx · components/chat" and "MessagesTimeline.tsx · src/components", making them indistinguishable to users.
Also found in 1 other location(s)
apps/web/src/components/ChatMarkdown.browser.tsx:105
The test
disambiguates duplicate file basenames inline(lines 105-125) expects links with disambiguating parent suffixes likeMessagesTimeline.tsx · components/chat, but the implementation inChatMarkdown.tsxhad this disambiguation logic removed (thefileLinkParentSuffixByPathcomputation and usage were deleted). The test expects·separators and parent directory suffixes that are no longer generated, so this test will fail.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/components/ChatMarkdown.tsx around line 407:
The removal of `fileLinkParentSuffixByPath` breaks disambiguation for files with identical basenames. When two links point to different files named `MessagesTimeline.tsx` in different directories, both now display as `"MessagesTimeline.tsx"` instead of `"MessagesTimeline.tsx · components/chat"` and `"MessagesTimeline.tsx · src/components"`, making them indistinguishable to users.
Evidence trail:
git_diff MERGE_BASE..REVIEWED_COMMIT for apps/web/src/components/ChatMarkdown.tsx shows removal of `buildFileLinkParentSuffixByPath` function (lines ~259-310 in old file), removal of `fileLinkParentSuffixByPath` useMemo (old ~497-499), and removal of parentSuffix label logic (old ~513-516). New code at line 437-438 constructs label as `[fileLinkMeta.basename]` only. The MarkdownFileLink component (lines 272-398 in new file) renders `label` as visible text in <span> and uses `displayPath` only in a TooltipPopup on hover.
Also found in 1 other location(s):
- apps/web/src/components/ChatMarkdown.browser.tsx:105 -- The test `disambiguates duplicate file basenames inline` (lines 105-125) expects links with disambiguating parent suffixes like `MessagesTimeline.tsx · components/chat`, but the implementation in `ChatMarkdown.tsx` had this disambiguation logic removed (the `fileLinkParentSuffixByPath` computation and usage were deleted). The test expects ` · ` separators and parent directory suffixes that are no longer generated, so this test will fail.
sandersonstabo
force-pushed
the
pr/ui-polish-context-menu
branch
from
ff1c49d to
6892790
Compare
I think I've resolved all conflicts now, and I've done my last pass now, I think I'm finished now |
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 6892790500
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| const shouldForceClearActiveTurn = shouldApplyThreadLifecycle | ||
| ? false | ||
| : event.type === "turn.completed" && activeTurnId !== null; |
There was a problem hiding this comment.
The new shouldForceClearActiveTurn condition fires for any blocked turn.completed event, including the normal out-of-order case where an older turn finishes after a newer turn has already become active (conflictsWithActiveTurn). In that scenario this branch clears activeTurnId and applies the stale event’s status, which can incorrectly mark an in-progress turn as finished/error and desynchronize thread lifecycle state.
Useful? React with 👍 / 👎.
| } else if (activeTurnId !== null) { | ||
| yield* Effect.logWarning( | ||
| "provider runtime ingestion: lifecycle guard blocked runtime.error but active turn is stuck — force-clearing", | ||
| { |
There was a problem hiding this comment.
When runtime.error is blocked by the lifecycle guard due to a turn-id mismatch, this new fallback still force-clears the active turn and sets session status to error. Delayed errors from a previous turn can therefore terminate and mislabel a currently running turn in the same thread, even though the guard correctly identified the event as unrelated to the active turn.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
🟢 Low
If all options have label "other" (case-insensitive), withCustomUserInputOption filters them all out after the options.length === 0 check on line 337. This causes the function to return a question with an empty options array, violating the expectation that questions have at least one option. Move the options.length === 0 check to after the withCustomUserInputOption call, or check the filtered result.
Also found in 1 other location(s)
apps/server/src/provider/acp/CursorAcpExtension.ts:66
If all options from Cursor have a label matching "other" (case-insensitive, trimmed),
withCustomUserInputOptionwill filter them all out, resulting in an emptyoptionsarray. The fallback[{ label: "OK", description: "Continue" }]only triggers whenquestion.options.length === 0before filtering, not after. This can produce aUserInputQuestionwith zero selectable options, likely causing UI issues or preventing the user from responding.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/server/src/provider/Layers/CodexAdapter.ts around line 319:
If all options have label "other" (case-insensitive), `withCustomUserInputOption` filters them all out after the `options.length === 0` check on line 337. This causes the function to return a question with an empty `options` array, violating the expectation that questions have at least one option. Move the `options.length === 0` check to after the `withCustomUserInputOption` call, or check the filtered result.
Evidence trail:
apps/server/src/provider/Layers/CodexAdapter.ts lines 319-349 (function toUserInputQuestions): options.length === 0 check at line 337 occurs before withCustomUserInputOption call at line 344.
apps/server/src/provider/userInputOptions.ts lines 1-7: withCustomUserInputOption filters out all options where label.trim().toLowerCase() === 'other', potentially returning an empty array.
Also found in 1 other location(s):
- apps/server/src/provider/acp/CursorAcpExtension.ts:66 -- If all options from Cursor have a label matching "other" (case-insensitive, trimmed), `withCustomUserInputOption` will filter them all out, resulting in an empty `options` array. The fallback `[{ label: "OK", description: "Continue" }]` only triggers when `question.options.length === 0` *before* filtering, not after. This can produce a `UserInputQuestion` with zero selectable options, likely causing UI issues or preventing the user from responding.
| function ProviderCustomColorPanel(props: { | ||
| readonly value: string; | ||
| readonly onCommit: (value: string) => void; | ||
| }) { | ||
| const { onCommit } = props; | ||
| const initialHsv = useMemo(() => hexToHsv(props.value), [props.value]); | ||
| const [hsv, setHsv] = useState(initialHsv); | ||
| const currentColor = hsvToHex(hsv.h, hsv.s, hsv.v); |
There was a problem hiding this comment.
🟢 Low settings/ProviderAccentColorPicker.tsx:83
ProviderCustomColorPanel initializes hsv state from initialHsv, but useState only uses the initial value on first mount. If props.value changes while the popover is open (e.g., external state update), the hsv state stays stale and the color picker displays the wrong color. Consider adding a useEffect that calls setHsv(hexToHsv(props.value)) whenever props.value changes.
+ useEffect(() => {
+ setHsv(hexToHsv(props.value));
+ }, [props.value]);
+
const currentColor = hsvToHex(hsv.h, hsv.s, hsv.v);🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/components/settings/ProviderAccentColorPicker.tsx around lines 83-90:
`ProviderCustomColorPanel` initializes `hsv` state from `initialHsv`, but `useState` only uses the initial value on first mount. If `props.value` changes while the popover is open (e.g., external state update), the `hsv` state stays stale and the color picker displays the wrong color. Consider adding a `useEffect` that calls `setHsv(hexToHsv(props.value))` whenever `props.value` changes.
Evidence trail:
apps/web/src/components/settings/ProviderAccentColorPicker.tsx lines 88-89 (useMemo + useState without useEffect sync), lines 83-86 (ProviderCustomColorPanel props), lines 201-203 (external value sync via useEffect in ProviderAccentColorPicker), apps/web/src/components/ui/popover.tsx (uses @base-ui/react/popover with Portal)
Co-authored-by: codex <codex@users.noreply.github.com> # Conflicts: # apps/web/src/components/chat/MessagesTimeline.logic.ts # apps/web/src/components/chat/MessagesTimeline.tsx # apps/web/src/components/settings/ProviderInstanceCard.tsx # apps/web/src/components/settings/SettingsPanels.tsx # apps/web/src/components/settings/settingsLayout.tsx
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 2 potential issues.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 6d9d3be. Configure here.
| detail: fallback, | ||
| cause: error, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Fully duplicated utility functions across two files
Medium Severity
Seven exported functions in apps/server/src/git/Utils.ts (toJsonSchemaObject, limitSection, extractJsonObject, sanitizeCommitSubject, sanitizePrTitle, sanitizeThreadTitle, normalizeCliError) are exact duplicates of functions already in apps/server/src/textGeneration/TextGenerationUtils.ts. The existing callers all import from TextGenerationUtils.ts, making the copies in git/Utils.ts unused dead code. Only sanitizeToolWorkLogSummaryLine is unique and actually imported from git/Utils.ts.
Reviewed by Cursor Bugbot for commit 6d9d3be. Configure here.
| options: ReadonlyArray<UserInputQuestionOption>, | ||
| ): ReadonlyArray<UserInputQuestionOption> { | ||
| return options.filter((option) => option.label.trim().toLowerCase() !== "other"); | ||
| } |
There was a problem hiding this comment.
Function name contradicts its filtering-only behavior
Low Severity
withCustomUserInputOption implies it adds a custom option to the list (the "with" prefix typically means "including"), but the implementation only removes options labeled "Other". This naming inversion makes it easy for future callers to misunderstand the function's purpose — it strips provider-supplied "Other" entries so the UI can supply its own custom input mechanism separately.
Reviewed by Cursor Bugbot for commit 6d9d3be. Configure here.
| @@ -648,20 +617,44 @@ export function ProviderInstanceCard({ | |||
| ); | |||
|
|
|||
| const authRowNode = ( | |||
There was a problem hiding this comment.
🟢 Low settings/ProviderInstanceCard.tsx:619
When a provider is authenticated with an email address, summary.detail (containing server messages like rate limits or deprecation notices) is no longer rendered. The previous code showed summary.detail for all providers, but the refactoring moved the detail popover inside the hasAuthenticatedEmail === false branch, so authenticated users never see these messages.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/components/settings/ProviderInstanceCard.tsx around line 619:
When a provider is authenticated with an email address, `summary.detail` (containing server messages like rate limits or deprecation notices) is no longer rendered. The previous code showed `summary.detail` for all providers, but the refactoring moved the detail popover inside the `hasAuthenticatedEmail === false` branch, so authenticated users never see these messages.
Evidence trail:
1. Old code diff (MERGE_BASE): `{summary.detail ? <span>- {summary.detail}</span> : null}` was outside both ternary branches at line ~668 of old file. See git_diff output for `apps/web/src/components/settings/ProviderInstanceCard.tsx`.
2. New code (REVIEWED_COMMIT): lines 619-660 of `ProviderInstanceCard.tsx` show `summary.detail` Popover is inside `hasAuthenticatedEmail === false` branch only.
3. `apps/web/src/components/settings/providerStatus.ts` lines 62-70: `getProviderSummary()` returns `detail: provider.message ? punctuateProviderMessage(provider.message) : null` when `provider.auth.status === 'authenticated'`, confirming `detail` can be non-null for authenticated providers.
| useEffect(() => { | ||
| const workEntry = workEntryRef.current; | ||
| const modelSelection = modelSelectionRef.current; | ||
|
|
||
| if (!toolCallSummaries || !cwd || !shouldSummarizeWorkLogEntry(workEntry)) { | ||
| setLine(null); | ||
| return; | ||
| } | ||
|
|
||
| const key = cacheKey(environmentId, workEntry.id, modelKey); |
There was a problem hiding this comment.
🟡 Medium hooks/useToolWorkLogFriendlyLine.ts:122
When workEntry changes (e.g., different id), the line state is not reset to null before the new async load begins. If the same component instance receives a different workEntry prop (common in virtualized/recycled lists), the previously loaded summary string remains visible for the new entry until the fetch completes, causing a brief flash of incorrect summary text.
useEffect(() => {
const workEntry = workEntryRef.current;
const modelSelection = modelSelectionRef.current;
if (!toolCallSummaries || !cwd || !shouldSummarizeWorkLogEntry(workEntry)) {
setLine(null);
return;
}
+ setLine(null);
const key = cacheKey(environmentId, workEntry.id, modelKey);🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/hooks/useToolWorkLogFriendlyLine.ts around lines 122-131:
When `workEntry` changes (e.g., different `id`), the `line` state is not reset to `null` before the new async load begins. If the same component instance receives a different `workEntry` prop (common in virtualized/recycled lists), the previously loaded summary string remains visible for the new entry until the fetch completes, causing a brief flash of incorrect summary text.
Evidence trail:
File: apps/web/src/hooks/useToolWorkLogFriendlyLine.ts at REVIEWED_COMMIT. Line ~100: `const [line, setLine] = useState<string | null>(null);` — initial state is null. Lines ~122-160: The useEffect depends on `requestSignature` (which changes when workEntry changes). In the normal (non-early-return) path, `setLine` is only called inside `.then()` and `.catch()` of the async `loadFriendlyLine()`. No `setLine(null)` is called at the top of the effect before kicking off the async load, so the previous `line` value persists until the new promise settles.
There was a problem hiding this comment.
🟡 Medium
The test on lines 105-125 expects ChatMarkdown to disambiguate duplicate file basenames by showing parent directory suffixes (e.g., MessagesTimeline.tsx · components/chat vs MessagesTimeline.tsx · src/components), but the implementation removed fileLinkParentSuffixByPath in the diff, so both links now render as just MessagesTimeline.tsx without disambiguation. This causes the test to fail because the · suffix logic no longer exists.
🤖 Copy this AI Prompt to have your agent fix this:
In file apps/web/src/components/ChatMarkdown.browser.tsx around line 105:
The test on lines 105-125 expects `ChatMarkdown` to disambiguate duplicate file basenames by showing parent directory suffixes (e.g., `MessagesTimeline.tsx · components/chat` vs `MessagesTimeline.tsx · src/components`), but the implementation removed `fileLinkParentSuffixByPath` in the diff, so both links now render as just `MessagesTimeline.tsx` without disambiguation. This causes the test to fail because the `·` suffix logic no longer exists.
Evidence trail:
1. apps/web/src/components/ChatMarkdown.browser.tsx lines 105-125 at REVIEWED_COMMIT: test expects links with names 'MessagesTimeline.tsx · components/chat' and 'MessagesTimeline.tsx · src/components'
2. git_diff MERGE_BASE..REVIEWED_COMMIT on apps/web/src/components/ChatMarkdown.tsx: `buildFileLinkParentSuffixByPath` function entirely deleted (~line 289-340), `fileLinkParentSuffixByPath` useMemo removed, parentSuffix logic in label construction removed, separator changed from ' · ' to ' @ '
3. git_grep for 'fileLinkParentSuffixByPath' at REVIEWED_COMMIT returns no results — the function no longer exists anywhere in the codebase
|
Any update on this @juliusmarminge? |
3 participants
Polished the UI.
Checks
Note
Polish chat UI, add LLM-generated tool work log summaries, and surface Claude auth status
summarizeToolWorkLogRPC endpoint backed by Claude, Codex, Cursor, and OpenCode providers; results are cached in IndexedDB and deduplicated in-flight viauseToolWorkLogFriendlyLine.TraitsPickerinto per-descriptor independent triggers; 'fastMode' is renamed to 'Speed' with Fast/Normal labels and rabbit/turtle icons; thinking-like booleans get a direct toggle button.MessagesTimelinerows: timestamps move to tooltips, copy buttons use ghost style, work log sections hide neutral entries and show success/failure/neutral indicators with expand/collapse, and the completion divider shows elapsed duration.ModelPickerContent; background scroll is suppressed while the picker is open.claude auth statusand surfaces it as a provider error banner.toolCallSummariessetting.MAX_VISIBLE_WORK_LOG_ENTRIESdrops from 6 to 1, so only the most recent work log entry is shown by default.Macroscope summarized 6d9d3be.
Note
Medium Risk
Adds new LLM-powered text-generation path exposed via WebSocket RPC and adjusts runtime session lifecycle handling, which can affect chat state correctness and provider interactions. Most other changes are UI polish, but the new summarization and turn-clearing logic warrant extra verification across providers.
Overview
Tool work-log summaries: Introduces
generateToolWorkLogSummaryacross all text-generation backends (Claude/Codex/Cursor/OpenCode), adds prompt/schema plumbing + sanitization helpers, and exposes it via a newWS_METHODS.gitSummarizeToolWorkLogRPC; desktop/web settings now include an opt-intoolCallSummariesflag (tests/layers updated accordingly).Provider/runtime behavior tweaks: Improves provider runtime ingestion guards to force-clear stuck active turns when a
turn.completedarrives without a turn id, and ensuresruntime.errorclearsactiveTurnId; Claude provider probing now checksclaude auth statusto surface an explicit unauthenticated error state.UI polish: Renames provider option label “Fast Mode” → “Speed”, filters provider-supplied
Otheruser-input options, enables OpenCode interaction-mode toggle, and refines multiple chat/git surfaces (branch picker UX, commit dialog redesign with diff stats, markdown file-link labels, shortcut styling, diff/plan/sidebar styling, and self-hosted fonts).Reviewed by Cursor Bugbot for commit 6d9d3be. Bugbot is set up for automated code reviews on this repo. Configure here.